Skip to content

chore(access): helper cleanup#4842

Merged
waleedlatif1 merged 1 commit into
stagingfrom
fix/access-cleanup-helper
Jun 2, 2026
Merged

chore(access): helper cleanup#4842
waleedlatif1 merged 1 commit into
stagingfrom
fix/access-cleanup-helper

Conversation

@icecrasher321
Copy link
Copy Markdown
Collaborator

@icecrasher321 icecrasher321 commented Jun 2, 2026

Summary

Helper cleanup for access checks.

Type of Change

  • Other: Code cleanup

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jun 2, 2026 1:03am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented Jun 2, 2026

PR Summary

Medium Risk
Touches authorization paths for knowledge APIs; behavior should be equivalent but refactors in security-sensitive code warrant careful review.

Overview
Refactors knowledge API access helpers in utils.ts by introducing internal resolveKnowledgeBaseAccess, resolveDocumentAccess, and resolveChunkAccess functions that take a requireWrite flag, so read vs write rules live in one place instead of duplicated DB/permission blocks.

The existing exports (checkKnowledgeBaseAccess, checkKnowledgeBaseWriteAccess, checkDocumentAccess, checkDocumentWriteAccess, checkChunkAccess, checkChunkWriteAccess) are unchanged in name and intended behavior: workspace KBs still allow any workspace permission for read and write/admin for write; legacy KBs still grant the owner in both modes. Document resolution now uses a full .select() on documents instead of an explicit column list (same filters: not excluded, not archived/deleted).

Docs are clarified for read vs write and chunk access (completed processing still required).

Reviewed by Cursor Bugbot for commit cb3e095. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 2, 2026

Greptile Summary

This PR consolidates duplicated permission-check logic across checkKnowledgeBaseAccess/checkKnowledgeBaseWriteAccess, checkDocumentAccess/checkDocumentWriteAccess, and checkChunkAccess/checkChunkWriteAccess into three private resolve* helpers that accept a requireWrite: boolean flag. All public function signatures are unchanged, and the refactoring is a straightforward DRY improvement with no behavioral regressions.

  • Three private resolve* functions (resolveKnowledgeBaseAccess, resolveDocumentAccess, resolveChunkAccess) now encapsulate the shared DB query and permission logic, each delegated from the original public wrappers.
  • The resolveDocumentAccess consolidation changes the document DB query from an explicit column list to .select() (select all), aligning it with the pre-existing behavior in the read path; the result is still cast to DocumentData, so the returned shape is identical.

Confidence Score: 5/5

Safe to merge — the public API is unchanged and the permission logic is faithfully preserved through the refactoring.

The change is a straightforward DRY refactor: three duplicated code paths are collapsed into parameterised private helpers. All six public functions retain their original signatures and delegate correctly. The only non-trivial difference is that checkDocumentWriteAccess now uses .select() instead of an explicit column list, which matches the pre-existing read path and produces the same cast result — no data is lost or misrouted.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/api/knowledge/utils.ts Refactors six public access-check helpers into three private resolve* functions parameterised by a requireWrite flag, preserving all public API signatures and permission logic faithfully

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph Public API
        CKA[checkKnowledgeBaseAccess]
        CKWA[checkKnowledgeBaseWriteAccess]
        CDA[checkDocumentAccess]
        CDWA[checkDocumentWriteAccess]
        CCA[checkChunkAccess]
        CCWA[checkChunkWriteAccess]
    end

    subgraph Private Resolvers
        RKBA["resolveKnowledgeBaseAccess\n(requireWrite: boolean)"]
        RDA["resolveDocumentAccess\n(requireWrite: boolean)"]
        RCA["resolveChunkAccess\n(requireWrite: boolean)"]
    end

    CKA -- "false" --> RKBA
    CKWA -- "true" --> RKBA
    CDA -- "false" --> RDA
    CDWA -- "true" --> RDA
    CCA -- "false" --> RCA
    CCWA -- "true" --> RCA

    RDA --> RKBA
    RCA --> RKBA

    RKBA --> |"requireWrite=false\nuserPermission !== null"| AccessGranted1[Access Granted]
    RKBA --> |"requireWrite=true\nwrite or admin"| AccessGranted2[Access Granted]
    RKBA --> AccessDenied1[Access Denied]
Loading

Reviews (1): Last reviewed commit: "chore(access): helper cleanup" | Re-trigger Greptile

@waleedlatif1 waleedlatif1 merged commit c786ada into staging Jun 2, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/access-cleanup-helper branch June 2, 2026 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants